Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[8.13](backport #38776) [Auditbeat] fim(kprobes): enrich file events by coupling add_process_metadata processor #38916

Closed
wants to merge 1 commit into from
Closed

[8.13](backport #38776) [Auditbeat] fim(kprobes): enrich file events by coupling add_process_metadata processor #38916

wants to merge 1 commit into from

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented Apr 13, 2024 •
edited by zube bot
Loading

Proposed commit message

This PR adds reporting of process.group.id, process.group.name and process.entity_id in add_process_metadata processor. Also it changes the factory of MetricSets to allow the latter to specify Processors after successful instantiation; this is required as FIM has 3 different available backends, namely fsnotify, kprobes, ebpf and only the kprobes one requires to have add_process_metadata processor. Utilising the former, kprobes backend always adds a properly configured add_process_metadata processor. As a result, enriching kprobes file events with process-related data exhibits the same robustness levels of the current add_process_metadata processor. However, the current design is aligned with @nick-alayil and the sec-linux-platform which plan to increase the robustness of add_process_metadata processor in a separate effort.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Author's Checklist

How to test this PR locally

Related issues

Screenshots

image
This is an automatic backport of pull request #38776 done by [Mergify](https://mergify.com).

@pkoutsovasilis @mergify
[Auditbeat] fim(kprobes): enrich file events by coupling add_process_…
ea54731 
…metadata processor (#38776)

* feat(processors/process_metadata): support reporting group id and name

* feat(processors/process_metadata): support reporting process entity_id

* feat(fim/kprobes): allow metricsSets to expose beat processors after initialisation

* doc: update CHANGELOG.next.asciidoc

* fix(linter): SA1015 prevent leaking the ticker

* fix(linter): SA1019 mark metricbeat/mb deprecation warnings that are not removed yet

* fix(linter): check for return err

* fix(linter): prealloc slices

* fix(linter): remove unused field

* fix(linter): G601 prevent implicit memory aliasing in for loop

* doc: update CHANGELOG.next.asciidoc

* fix: update filebaet fields.asciidoc (unrelated to this work)

* doc: remove irrelevant changes from CHANGELOG.next.asciidoc

* feat(processor/metadata): introduce new type based allocation func

* feat(fim/kprobe): instantiate new processor alongside a new kprobes event reader

* fix(fim): remove redundant whitespace

* doc(metricbeat): enrich documentation about Processors attached to a Metricbeat

* fix(fim): gofumpt eventreader_kprobes.go

* fix(add_process_metadata): gofmt add_process_metadata.go gosysinfo_provider.go

* fix(lint): goimports eventreader_kprobes.go

* fix(winlogbeat): generate include list [unrelated to this PR]

(cherry picked from commit ca4adce)

# Conflicts:
#	libbeat/processors/add_process_metadata/add_process_metadata.go
#	libbeat/processors/add_process_metadata/config.go
#	libbeat/processors/add_process_metadata/gosysinfo_provider.go
#	metricbeat/mb/module/configuration.go
@mergify mergify bot requested a review from a team as a code owner April 13, 2024 02:41
@mergify mergify bot added backport conflicts There is a conflict in the backported pull request labels Apr 13, 2024
@mergify mergify bot requested review from a team as code owners April 13, 2024 02:41
@mergify Mergify
Copy link
Contributor Author

mergify bot commented Apr 13, 2024

Cherry-pick of ca4adce has failed:

On branch mergify/bp/8.13/pr-38776
Your branch is up to date with 'origin/8.13'.

You are currently cherry-picking commit ca4adcecac.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   CHANGELOG.next.asciidoc
	modified:   auditbeat/module/file_integrity/eventreader_kprobes.go
	modified:   auditbeat/module/file_integrity/eventreader_linux.go
	modified:   auditbeat/module/file_integrity/metricset.go
	modified:   libbeat/processors/add_process_metadata/add_process_metadata_test.go
	modified:   libbeat/processors/add_process_metadata/cache.go
	modified:   libbeat/processors/add_process_metadata/cache_test.go
	modified:   metricbeat/mb/module/connector.go
	modified:   metricbeat/mb/module/connector_test.go
	modified:   metricbeat/mb/module/factory.go
	modified:   metricbeat/mb/module/runner_group.go
	modified:   metricbeat/mb/module/runner_group_test.go
	modified:   metricbeat/mb/module/testing.go
	modified:   metricbeat/mb/module/wrapper.go
	modified:   x-pack/winlogbeat/include/list.go

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   libbeat/processors/add_process_metadata/add_process_metadata.go
	both modified:   libbeat/processors/add_process_metadata/config.go
	both modified:   libbeat/processors/add_process_metadata/gosysinfo_provider.go
	both modified:   metricbeat/mb/module/configuration.go

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@mergify mergify bot requested review from ycombinator and removed request for a team April 13, 2024 02:41
@mergify mergify bot requested a review from belimawr April 13, 2024 02:41
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Apr 13, 2024
@botelastic
Copy link

botelastic bot commented Apr 13, 2024

This pull request doesn't have a Team:<team> label.

@elasticmachine
Copy link
Collaborator

💔 Build Failed

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2024-04-13T02:41:22.721+0000

  • Duration: 76 min 16 sec

Test stats 🧪

Test Results
Failed 0
Passed 1493
Skipped 146
Total 1639

Steps errors 196

Expand to view the steps failures

Show only the first 10 steps failures

x-pack/winlogbeat-build-windows-2019 - mage build unitTest
  • Took 3 min 34 sec . View more details here
  • Description: mage build unitTest
x-pack/winlogbeat-build-windows-2019 - mage build unitTest
  • Took 0 min 29 sec . View more details here
  • Description: mage build unitTest
x-pack/winlogbeat-build-windows-2019 - mage build unitTest
  • Took 0 min 28 sec . View more details here
  • Description: mage build unitTest
x-pack/winlogbeat-windows-2022-windows-2022 - mage build unitTest
  • Took 2 min 23 sec . View more details here
  • Description: mage build unitTest
x-pack/winlogbeat-windows-2022-windows-2022 - mage build unitTest
  • Took 0 min 23 sec . View more details here
  • Description: mage build unitTest
x-pack/winlogbeat-windows-2022-windows-2022 - mage build unitTest
  • Took 0 min 28 sec . View more details here
  • Description: mage build unitTest
x-pack/winlogbeat-windows-2016-windows-2016 - mage build unitTest
  • Took 3 min 31 sec . View more details here
  • Description: mage build unitTest
x-pack/winlogbeat-windows-2016-windows-2016 - mage build unitTest
  • Took 1 min 30 sec . View more details here
  • Description: mage build unitTest
x-pack/winlogbeat-windows-2016-windows-2016 - mage build unitTest
  • Took 1 min 31 sec . View more details here
  • Description: mage build unitTest
Error signal
  • Took 0 min 0 sec . View more details here
  • Description: Error 'hudson.AbortException: script returned exit code 1'

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

andrewkroh
andrewkroh requested changes Apr 13, 2024
View reviewed changes
Copy link
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought this is a new feature for 8.14?

@pkoutsovasilis
Copy link
Contributor

I thought this is a new feature for 8.14?

kprobes FIM BE is part of 8.13 so this enhancement can go in 8.13.x series?

@pkoutsovasilis
Copy link
Contributor

do you think we should skip backporting this to 8.13 @andrewkroh? 🙂

@andrewkroh
Copy link
Member

Ok, I see where you are coming from. With this being an enhancement instead of a pure bugfix I would prefer to leave it for the next minor version rather than including it in the next 8.13.x patch. Sound good?

I try to follow https://semver.org/ as much as possible. Sometimes there are exceptions, but I don't think this is one. Plus there isn't another 8.13.x planned.

@pkoutsovasilis
Copy link
Contributor

Ok, I see where you are coming from. With this being an enhancement instead of a pure bugfix I would prefer to leave it for the next minor version rather than including it in the next 8.13.x patch. Sound good?

I try to follow https://semver.org/ as much as possible. Sometimes there are exceptions, but I don't think this is one. Plus there isn't another 8.13.x planned.

Following semver as close as possible, music to my ears 🎶! I totally agree with you @andrewkroh , thanks for catching that. Let's leave this available only for 8.14

@mergify mergify bot deleted the mergify/bp/8.13/pr-38776 branch April 13, 2024 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrewkroh andrewkroh andrewkroh requested changes

@ycombinator ycombinator Awaiting requested review from ycombinator ycombinator is a code owner automatically assigned from elastic/elastic-agent-data-plane

@belimawr belimawr Awaiting requested review from belimawr belimawr is a code owner automatically assigned from elastic/elastic-agent-data-plane

Assignees

@pkoutsovasilis pkoutsovasilis

Labels
backport conflicts There is a conflict in the backported pull request needs_team Indicates that the issue/PR needs a Team:* label
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@elasticmachine @pkoutsovasilis @andrewkroh